Added HEC error messages #36871
Added HEC error messages #36871
Conversation
|
Had to close the previous PR because of messy commit history, so reopening here. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Co-authored-by: Antoine Toulme <[email protected]>
|
Let's resolve the discussion #35712 (comment) first |
Hi @dmitryax, I responded to the discussion you linked, any updates? |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
|
Sorry for the delay. My goal was to avoid recreating the body. I’ll try to quickly prototype my suggestion |
| unmarshalError := json.Unmarshal(body, &jsonResponse) | ||
|
|
||
| if unmarshalError == nil { | ||
| responseErrorValue, ok := jsonResponse["text"] |
There was a problem hiding this comment.
This function is being used by the Signalfx exporter, not only the Splunk HEC exporter. The SignalFx exporter probably doesn't return JSON with the text field, so it'll be a redundant step in that add an unnecessary error message from Splunk found to all errors.
Later in this function, we dump the whole payload for http.StatusBadRequest and http.StatusUnauthorized. Why can't we just keep doing that? If you want to extend the same behavior for some other error codes that are expected to go along with the body, feel free to do that
There was a problem hiding this comment.
The problem with that is that httputil.DumpResponse only returns a generic error - like HTTP 403 Forbidden. If we want to display messages returned by splunk (https://docs.splunk.com/Documentation/Splunk/9.2.0/Data/TroubleshootHTTPEventCollector#Possible_error_codes) we have to actually read the body.
For the unnecessary message, I guess it would make sense to just skip this condition then?
Description
Errors returned from Splunk HEC will be displayed in logs. This will help discover some issues without extensive debugging.
Testing
Unit testing and manual testing of the feature